-
-
Notifications
You must be signed in to change notification settings - Fork 32.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[core] Update slot components to use overridesResolver part 1 #25853
[core] Update slot components to use overridesResolver part 1 #25853
Conversation
const { styleProps } = props; | ||
|
||
return { | ||
[`& .${accordionClasses.region}`]: styles.region, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no component for this one.
const overridesResolver = (props, styles) => { | ||
return deepmerge( | ||
{ | ||
[`& .${avatarGroupClasses.avatar}`]: styles.avatar, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a good example where this approach may not work everywhere. In this case we need to still keep this, otherwise it won't work.
[`& .${autocompleteClasses.tag}`]: { | ||
...styles.tag, | ||
...styles[`tagSize${capitalize(size)}`], | ||
}, | ||
[`& .${autocompleteClasses.inputRoot}`]: styles.inputRoot, | ||
[`& .${autocompleteClasses.input}`]: { | ||
...styles.input, | ||
...(inputFocused && styles.inputFocused), | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are no slot components for these overrides keys, so we need to keep them
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good
👍 ok let's continue with the rest of the components. |
This PR updates the components starting with
A
to useoverridesResolver
per slot component. (need to somehow batch the changes in few PRs, otherwise the PR would be too big).I am disabling the
testDeepOverrides
as we no longer support it. In some cases we still need to define the overrides on the root, for example in theAvatarGroup
, when we are just applying the classes on a component down the tree, which is not created by astyled()
utility.I've done some preliminary benchmark in #25850 and seems like perf are great with this approach.